components/webui/server/src/routes/api/presto-search/index.ts (2)

59-76: Rows are dropped when data arrives before searchJobId is resolved. Buffer and flush.

Current logic returns early and skips inserts if data arrives before state sets searchJobId. This can lose initial result batches.

Refactor to buffer batches until queryId is known, then flush:

             try {
                 searchJobId = await new Promise<string>((resolve, reject) => {
-                    let isResolved = false;
+                    let isResolved = false;
+                    const pendingBatches: Array<{rows: unknown[]; columns: unknown[]}> = [];
                     Presto.client.execute({
                         // ...
-                        data: (_, data, columns) => {
+                        data: (_, data, columns) => {
                             request.log.info(
                                 `Received ${data.length} rows from Presto query`
                             );
 
-                            if (false === isResolved) {
-                                request.log.error(
-                                    "Presto data received before searchJobId was resolved; " +
-                                    "skipping insert."
-                                );
-
-                                return;
-                            }
-
                             if (0 === data.length) {
                                 return;
                             }
 
-                            insertPrestoRowsToMongo(
-                                data,
-                                columns,
-                                searchJobId,
-                                mongoDb
-                            ).catch((err: unknown) => {
-                                request.log.error(
-                                    err,
-                                    "Failed to insert Presto results into MongoDB"
-                                );
-                            });
+                            if (false === isResolved) {
+                                // Buffer until we know the queryId/collection
+                                pendingBatches.push({rows: data, columns});
+                                return;
+                            }
+
+                            insertPrestoRowsToMongo(data, columns, searchJobId, mongoDb)
+                                .catch((err: unknown) => {
+                                    request.log.error(err, "Failed to insert Presto results into MongoDB");
+                                });
                         },
                         // ...
                         state: (_, queryId, stats) => {
                             request.log.info({
                                 searchJobId: queryId,
                                 state: stats.state,
                             }, "Presto search state updated");
 
                             if (false === isResolved) {
                                 isResolved = true;
-                                resolve(queryId);
+                                resolve(queryId);
+                                // Flush buffered batches
+                                if (0 < pendingBatches.length) {
+                                    void Promise.allSettled(
+                                        pendingBatches.map((b) =>
+                                            insertPrestoRowsToMongo(b.rows, b.columns, queryId, mongoDb)
+                                        )
+                                    ).then((results) => {
+                                        for (const r of results) {
+                                            if ("rejected" === r.status) {
+                                                request.log.error(r.reason, "Failed to flush buffered Presto rows to MongoDB");
+                                            }
+                                        }
+                                    });
+                                    pendingBatches.length = 0;
+                                }
                             }
                         },

Also applies to: 77-88, 103-107

---

90-94: Log failures at error level, not info.

Use error severity for failures to aid observability and alerting.

-                        error: (error) => {
-                            request.log.info(error, "Presto search failed");
+                        error: (error) => {
+                            request.log.error(error, "Presto search failed");

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (2)

5-8: Normalise the import path.

".././../../../config" is equivalent to "../../../../config" but noisier. Use the simpler relative path.

-import {
-    CLP_STORAGE_ENGINES,
-    SETTINGS_STORAGE_ENGINE,
-} from ".././../../../config";
+import {CLP_STORAGE_ENGINES, SETTINGS_STORAGE_ENGINE} from "../../../../config";

---

63-66: Remove unintended leading space in fallback string

The string literal " Original file" at
components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx
includes a stray leading space, which will render extra whitespace in the UI. Update it to remove the space and, if you need additional spacing, handle that via CSS or layout rather than embedded literal whitespace.

• File: components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/typings.tsx (around line 65)

-                        " Original file"
+                        "Originalfile"

components/webui/client/src/pages/SearchPage/SearchControls/Presto/presto-search-requests.ts (1)

13-28: Return the promise to enable upstream flow control and testing.

Let callers await the submission, chain UI side-effects, or handle errors centrally.

-const handlePrestoQuerySubmit = (payload: PrestoQueryJobCreationSchema) => {
-    const store = useSearchStore.getState();
-    submitQuery(payload)
-        .then((result) => {
-            const {searchJobId} = result.data;
-            store.updateSearchJobId(searchJobId);
-            console.debug(
-                "Presto search job created - ",
-                "Search job ID:",
-                searchJobId
-            );
-        })
-        .catch((err: unknown) => {
-            console.error("Failed to submit query:", err);
-        });
-};
+const handlePrestoQuerySubmit = async (payload: PrestoQueryJobCreationSchema) => {
+    const store = useSearchStore.getState();
+    try {
+        const result = await submitQuery(payload);
+        const {searchJobId} = result.data;
+        store.updateSearchJobId(searchJobId);
+        console.debug("Presto search job created - ", "Search job ID:", searchJobId);
+        return result;
+    } catch (err) {
+        console.error("Failed to submit query:", err);
+        throw err;
+    }
+};

components/webui/server/src/routes/api/presto-search/utils.ts (2)

8-14: Update JSDoc to reflect nested {row: ...} structure.

The function now wraps columns under row, but the doc still describes a flat object. Please update to avoid confusion.

Apply this diff:

- * @return An object mapping each column name to its corresponding value from the row.
+ * @return An object with a single `row` property that maps each column name to its corresponding value.

---

15-25: Tighten types for better safety and IDE support.

Return type is currently a broad Record<string, unknown>, and the insert result drops the schema generic. Define a concrete doc schema and use it end-to-end.

Apply this diff:

@@
-const prestoRowToObject = (
+type PrestoRowDoc = {row: Record<string, unknown>};
+
+const prestoRowToObject = (
     row: unknown[],
     columns: {name: string}[]
-): Record<string, unknown> => {
+): PrestoRowDoc => {
     const obj: Record<string, unknown> = {};
     columns.forEach((col, idx) => {
         obj[col.name] = row[idx];
     });
 
-    return {row: obj};
+    return {row: obj};
 };
@@
-): Promise<InsertManyResult> => {
+): Promise<InsertManyResult<PrestoRowDoc>> => {
     const collection = mongoDb.collection(searchJobId);
     const resultDocs = data.map((row) => prestoRowToObject(row, columns));
     return collection.insertMany(resultDocs);
 }

Also applies to: 41-45

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Learnt from: haiqi96
PR: y-scope/clp#0
File: :0-0
Timestamp: 2025-07-29T14:04:13.769Z
Learning: User haiqi96 requested creating a GitHub issue to document a bug fix from PR #1136, which addressed MySQL compatibility issues with invalid SQL CAST operations in the WebUI component.

Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.

Learnt from: junhaoliao
PR: y-scope/clp#939
File: components/package-template/src/etc/clp-config.yml:64-64
Timestamp: 2025-06-22T04:01:43.409Z
Learning: The new webui architecture uses Fastify with Pino logging instead of the previous Winston-based logging system that was removed during the webui refactoring.

Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.

Learnt from: junhaoliao
PR: y-scope/clp#937
File: components/log-viewer-webui/client/src/AntdApp.tsx:16-24
Timestamp: 2025-05-29T20:33:40.653Z
Learning: In components/log-viewer-webui React codebase: Return type annotations (like `: JSX.Element`) are unnecessary and not preferred for React components in JSX/TSX files.

Learnt from: davemarco
PR: y-scope/clp#797
File: components/log-viewer-webui/client/src/components/Layout/MainLayout.tsx:2-5
Timestamp: 2025-04-08T22:32:05.366Z
Learning: In this codebase using React Router v7.4.1, components should be imported directly from "react-router" (e.g., `import { Link, Outlet } from "react-router";`) rather than from "react-router-dom" as was common in previous versions of React Router.

Learnt from: hoophalab
PR: y-scope/clp#1108
File: components/webui/client/src/pages/SearchPage/SearchControls/Presto/SqlQueryInput/index.tsx:15-15
Timestamp: 2025-07-18T20:00:50.288Z
Learning: In the y-scope/clp React webui client codebase, for Zustand store usage: use `useStore.getState().method` for callbacks since the output is not reactive and doesn't need state as a dependency in the hook, and use `useStore((state) => state.property)` with proper selectors for reactive components that need to re-render when state changes.

Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/SearchResultsVirtualTable/typings.tsx (1)

63-66: Remove unintended leading space in fallback label.

The string literal has a leading space: " Original file". If not intentional for formatting, remove it.

Apply:

-                        " Original file"
+                        "Original file"

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/SearchResultsVirtualTable/useSearchResults.ts (1)

24-26: Avoid console.log in hooks; guard or use a central logger.

Leaving console.log in production UI can be noisy. Either wrap in an environment guard or remove.

Apply:

-            console.log(
-                `Subscribing to updates to search results with job ID: ${searchJobId}`
-            );
+            if (false == (process.env.NODE_ENV === "production")) {
+                // eslint-disable-next-line no-console
+                console.log(`Subscribing to search results for job ID: ${searchJobId}`);
+            }

components/webui/server/src/routes/api/presto-search/utils.ts (2)

9-16: Update JSDoc @return to reflect the wrapped document shape.

The function now returns { [PRESTO_DATA_PROPERTY]: obj }, not the plain mapping object. Keep docs accurate.

Apply:

- * @return An object mapping each column name to its corresponding value from the row.
+ * @return A Mongo-friendly document: { [PRESTO_DATA_PROPERTY]: { <columnName>: value, ... } }.

---

21-24: Consider validating row/column length alignment.

If row.length !== columns.length, some values will be undefined or dropped. Add a guard (log or throw) to catch upstream mismatches early.

For example:

 const prestoRowToObject = (
   row: unknown[],
   columns: {name: string}[]
 ): Record<string, unknown> => {
+  if (row.length !== columns.length) {
+    // Use your logger if available instead of console.warn
+    // eslint-disable-next-line no-console
+    console.warn("prestoRowToObject: row/columns length mismatch", {
+      rowLength: row.length,
+      columnsLength: columns.length,
+      columns: columns.map(c => c.name),
+    });
+  }
   const obj: Record<string, unknown> = {};
   columns.forEach((col, idx) => {
     obj[col.name] = row[idx];
   });

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (3)

4-4: Make PrestoSearchResult import type-only to avoid a runtime cycle

This mirrors the earlier feedback: importing the type as a value can create a runtime cycle with typings.tsx. Type-only import is erased at build time.

-import {PrestoSearchResult} from "./typings";
+import type {PrestoSearchResult} from "./typings";

---

23-30: Use PRESTO_DATA_PROPERTY for key extraction and fix the ESLint array-bracket-newline error

• Deriving keys from data[0].row hard-codes the property name; use the shared constant instead.
• The current dataIndex array formatting triggers the pipeline failure: “A linebreak is required before ']'”. Move ] to its own line (or keep the array on one line).

-    return Object.keys(data[0].row)
-        .map((key) => ({
-            dataIndex: [
-                PRESTO_DATA_PROPERTY,
-                key],
-            key: key,
-            title: key,
-        }));
+    const row = data[0][PRESTO_DATA_PROPERTY] as Record<string, unknown>;
+    return Object.keys(row)
+        .map((key) => ({
+            dataIndex: [
+                PRESTO_DATA_PROPERTY,
+                key
+            ],
+            key: key,
+            title: key,
+        }));

---

16-21: Use in with PRESTO_DATA_PROPERTY for the presence check (and keep the undefined-guard), matching codebase style

Avoid checking the property’s value via typeof ... === "undefined". The intention is to ensure the property exists. Also aligns with the coding guideline preferring false == <expr> to !<expr>.

-    if (0 === data.length ||
-        "undefined" === typeof data[0] ||
-        "undefined" === typeof data[0][PRESTO_DATA_PROPERTY]
-    ) {
+    if (0 === data.length ||
+        "undefined" === typeof data[0] ||
+        false == (PRESTO_DATA_PROPERTY in data[0])
+    ) {
         return [];
     }

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (4)

1-1: Make TableProps a type-only import to avoid a runtime edge

Prevents emitting a runtime import for a compile-time type.

-import {TableProps} from "antd";
+import type {TableProps} from "antd";

---

4-4: Mark PrestoSearchResult as a type-only import to prevent runtime cycle

typings.tsx re-exports a value from this utils file; importing the type as type-only avoids a module cycle at runtime.

-import {PrestoSearchResult} from "./typings";
+import type {PrestoSearchResult} from "./typings";

---

16-21: Use property-existence check with the shared constant; avoid typeof “undefined”

Simplifies the guard and follows the project style (false == …). Also aligns with the PRESTO_DATA_PROPERTY abstraction.

-    if (0 === data.length ||
-        "undefined" === typeof data[0] ||
-        "undefined" === typeof data[0][PRESTO_DATA_PROPERTY]
-    ) {
+    if (0 === data.length || false == (PRESTO_DATA_PROPERTY in data[0])) {
         return [];
     }

---

23-31: Avoid hard-coding “row”; use PRESTO_DATA_PROPERTY for key enumeration

The guard uses PRESTO_DATA_PROPERTY but enumeration still references .row. Use the constant consistently.

-    return Object.keys(data[0].row)
+    const row = data[0][PRESTO_DATA_PROPERTY] as Record<string, unknown>;
+    return Object.keys(row)
         .map((key) => ({
             dataIndex: [
                 PRESTO_DATA_PROPERTY,
                 key,
             ],
             key: key,
             title: key,
         }));

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

components/webui/server/src/routes/api/presto-search/utils.ts (2)

8-15: Update JSDoc to reflect the wrapped return shape

Doc still states it returns a plain mapping; it now returns a wrapper object.

 /**
- * Converts a Presto result row (array of values) into an object, using the provided column
- * definitions to assign property names.
+ * Converts a Presto result row (array of values) into a Mongo-ready wrapper object, using the
+ * provided column definitions to assign property names.
  *
  * @param row Array of values representing a single Presto result row.
  * @param columns Array of column definitions, each containing a `name` property.
- * @return An object mapping each column name to its corresponding value from the row.
+ * @returns A PrestoRowObject whose `row` maps each column name to its corresponding value.
  */

---

45-46: Annotate resultDocs for clearer intent and safer refactors

Explicitly typing the insertion payload avoids widening to any and future regressions if prestoRowToObject’s signature changes.

-    const resultDocs = data.map((row) => prestoRowToObject(row, columns));
+    const resultDocs: PrestoRowObject[] = data.map((row) => prestoRowToObject(row, columns));

components/webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/Presto/PrestoResultsVirtualTable/utils.ts (4)

1-1: Import TableProps as a type-only import

Prevents emitting a runtime import edge.

-import {TableProps} from "antd";
+import type {TableProps} from "antd";

---

3-3: Import PrestoSearchResult as a type-only import

Avoids creating a runtime cycle with typings.

-import {PrestoSearchResult} from "./typings";
+import type {PrestoSearchResult} from "./typings";

---

6-11: Complete the JSDoc returns description

Clarify what the function returns to aid maintainability and tooling.

 /**
  * Generates dynamic columns configuration for Presto query engine.
  *
  * @param data Array of Presto search results
- * @return
+ * @returns Columns configuration for Ant Design Table derived from the first result row
  */

---

15-20: Harden guard against null/non-object row payloads

Prevents runtime errors if row is null or malformed.

-    if (0 === data.length ||
-        "undefined" === typeof data[0] ||
-        "undefined" === typeof data[0].row
-    ) {
+    if (0 === data.length ||
+        "undefined" === typeof data[0] ||
+        "undefined" === typeof data[0].row ||
+        null === data[0].row ||
+        "object" !== typeof data[0].row
+    ) {
         return [];
     }

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

Learnt from: junhaoliao
PR: y-scope/clp#596
File: components/log-viewer-webui/client/src/api/query.js:16-23
Timestamp: 2024-11-21T15:51:33.203Z
Learning: In `components/log-viewer-webui/client/src/api/query.js`, the `ExtractJsonResp` type definition is accurate as-is and does not require modification. When suggesting changes to type definitions, ensure they align with the server-side definitions, referencing the source code if necessary.

components/webui/server/src/routes/api/presto-search/index.ts (1)

227-228: Make DELETE /results idempotent; ignore “NamespaceNotFound”.

Dropping a non-existent collection can throw and cause a 500. Treat it as success and keep returning 204.

Apply:

-            await mongoDb.collection(searchJobId).drop();
+            try {
+                const wasDropped = await mongoDb.collection(searchJobId).drop();
+                if (false === wasDropped) {
+                    request.log.warn({searchJobId}, "MongoDB collection did not exist; continuing");
+                }
+            } catch (err: unknown) {
+                const e = err as { codeName?: string; code?: number };
+                // Ignore "NamespaceNotFound" (code: 26) to keep the operation idempotent
+                if ("NamespaceNotFound" === e.codeName || 26 === e.code) {
+                    request.log.warn({searchJobId}, "MongoDB collection did not exist; continuing");
+                } else {
+                    request.log.error(err, "Failed to drop MongoDB collection for Presto search results");
+                    throw err;
+                }
+            }

components/webui/server/src/routes/api/presto-search/index.ts (1)

171-172: Make Mongo collection creation idempotent; handle “NamespaceExists” and keep 201.

There is a race window where the first insert can auto-create the collection before this createCollection runs, causing a “NamespaceExists” error and a 500. Make the creation defensive and non-fatal for the “already exists” case.

Apply:

-            await mongoDb.createCollection(searchJobId);
+            try {
+                await mongoDb.createCollection(searchJobId);
+            } catch (err: unknown) {
+                const e = err as { codeName?: string; code?: number };
+                // Ignore "collection already exists" (codeName: NamespaceExists, code: 48)
+                if ("NamespaceExists" === e.codeName || 48 === e.code) {
+                    request.log.warn({searchJobId}, "MongoDB collection already exists; continuing");
+                } else {
+                    request.log.error(err, "Failed to create MongoDB collection for Presto search results");
+                    throw err;
+                }
+            }

Also consider validating searchJobId against Mongo’s collection name rules (length, not empty, not starting with system., no NUL) since it originates from an external system.

components/webui/common/index.ts (1)

132-139: Make PrestoRowObject generic to preserve row shape end-to-end.

Allows callers to strongly type the contained row while preserving current behaviour via a default.

 /**
  * Presto row wrapped in a `row` property to prevent conflicts with MongoDB's `_id` field.
  */
-interface PrestoRowObject {
-    row: Record<string, unknown>;
-}
+interface PrestoRowObject<T extends Record<string, unknown> = Record<string, unknown>> {
+    row: T;
+}

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.